Simple RuntimeTypeSystem DacDbi APIs#126732
Merged
rcj1 merged 3 commits intodotnet:mainfrom Apr 10, 2026
Merged
Conversation
Contributor
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds minimal RuntimeTypeSystem contract surface and corresponding Legacy DacDbi/SOS implementations to support a few simple DacDbi APIs (HasTypeParams, GetTypeHandle, GetThreadStaticAddress) using cDAC data.
Changes:
- Implement
DacDbiImpl.HasTypeParams,GetTypeHandle, andGetThreadStaticAddressagainst cDAC contracts (with DEBUG cross-validation vs legacy DAC when available). - Extend RuntimeTypeSystem contract with
ContainsGenericVariablesandGetFieldDescThreadStaticAddress, plus supporting MethodTable flag plumbing. - Consolidate ECMA token helpers into
EcmaMetadataUtilsand update docs/HResults accordingly.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs | Replaces local metadata token enum usage with shared EcmaMetadataUtils token helpers. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs | Implements the three requested DacDbi APIs via cDAC contracts with DEBUG parity checks. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/RuntimeTypeSystemHelpers/MethodTableFlags_1.cs | Adds ContainsGenericVariables MethodTable flag bit support. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/EcmaMetadataUtils.cs | Makes token utilities publicly reusable (token type enum + mask). |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs | Implements ContainsGenericVariables and adds thread-static address computation API. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs | Adds HRESULT constant for “class not loaded”. |
| src/native/managed/cdac/docs/design/datacontracts/RuntimeTypeSystem.md | Documents the new RuntimeTypeSystem APIs and static/thread-static address helpers. |
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/CorDbHResults.cs
Outdated
Show resolved
Hide resolved
...ged/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs
Show resolved
Hide resolved
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs
Show resolved
Hide resolved
...ged/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs
Show resolved
Hide resolved
noahfalk
approved these changes
Apr 10, 2026
Member
noahfalk
left a comment
noahfalk
left a comment
There was a problem hiding this comment.
Copilot had some good feedback but other than that it looked good to me
max-charlamb
approved these changes
Apr 10, 2026
Member
max-charlamb
left a comment
max-charlamb
left a comment
There was a problem hiding this comment.
looks good. It'd be good to have some simple unit/dump tests
Use full dump until heap dumps are fixed.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeTypeSystem_1.cs:1902
- GetFieldDescStaticOrThreadStaticAddress uses
_target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!, butIEcmaMetadata.GetMetadatais nullable. In dump/minidump scenarios this can be null and would turn into a NullReferenceException rather than returningTargetPointer.Null(or a controlled HRESULT). Consider handling the null case explicitly before callingGetFieldDefinition.
MetadataReader mdReader = _target.Contracts.EcmaMetadata.GetMetadata(moduleHandle)!;
uint token = ((IRuntimeTypeSystem)this).GetFieldDescMemberDef(fieldDescPointer);
FieldDefinitionHandle fieldHandle = (FieldDefinitionHandle)MetadataTokens.Handle((int)token);
FieldDefinition fieldDef = mdReader.GetFieldDefinition(fieldHandle);
src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/Dbi/DacDbiImpl.cs
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementing DacDbi APIs
HasTypeParams,GetTypeHandleandGetThreadStaticAddressthat use and require minimal modifications to the RuntimeTypeSystem cDAC contract.